feat: Add API configuration support for summarization#37
Conversation
Add environment variables to configure summarization API settings: - EPISODIC_MEMORY_API_MODEL: Override model (default: haiku) - EPISODIC_MEMORY_API_BASE_URL: Custom Anthropic-compatible endpoint - EPISODIC_MEMORY_API_TOKEN: Auth token for custom endpoint - EPISODIC_MEMORY_API_TIMEOUT_MS: Timeout for slow endpoints This allows routing summarization through custom Anthropic-compatible endpoints or enterprise proxies without affecting interactive Claude sessions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughAdds API configuration support for episodic-memory summarization: new environment variables allow routing summarization to a custom Anthropic-compatible endpoint or overriding the model; summarizer injects these env overrides and can fall back to a secondary model; README and a test were added. Changes
Sequence Diagram(s)sequenceDiagram
participant Summarizer
participant ClaudeSDK as Claude SDK (Anthropic-compatible)
participant ExternalAPI as Custom Endpoint
Summarizer->>Summarizer: build env via getApiEnv() and choose model (primary/fallback)
Summarizer->>ClaudeSDK: summarizeConversation(request, {model: primary, env: getApiEnv()})
alt request goes to custom base URL
ClaudeSDK->>ExternalAPI: HTTP request (with overridden base URL & token)
ExternalAPI-->>ClaudeSDK: HTTP response (success or error)
else SDK direct
ClaudeSDK-->>Summarizer: response or error
end
alt thinking-budget / API error & fallback available
Summarizer->>Summarizer: switch to fallback model
Summarizer->>ClaudeSDK: summarizeConversation(request, {model: fallback, env: getApiEnv()})
ClaudeSDK-->>Summarizer: response or error
end
Summarizer-->>Caller: final summary or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dist/summarizer.jsis excluded by!**/dist/**
📒 Files selected for processing (2)
README.md(1 hunks)src/summarizer.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/summarizer.ts (1)
dist/summarizer.js (7)
baseUrl(13-13)token(14-14)timeoutMs(15-15)env(19-19)configuredModel(42-42)defaultModel(43-43)model(44-44)
🔇 Additional comments (3)
README.md (1)
143-143: Verify the timeout value is intentional.The example timeout of 3000000ms equals 50 minutes, which is unusually long for API calls. Confirm this is the intended value and not a typo (perhaps 300000ms = 5 minutes was intended?). While enterprise proxies or rate-limited endpoints might justify extended timeouts, 50 minutes seems extreme.
src/summarizer.ts (2)
46-48: LGTM: Clean model selection with environment override.The model selection logic correctly prioritizes the configured model from
EPISODIC_MEMORY_API_MODEL, falling back to the existing haiku/sonnet logic when not set.
50-62: Remove the type cast or document theenvparameter requirement.The
as anycast at line 62 appears justified—theenvparameter is not part of the officially documented TypeScript Claude Agent SDK query options. Either the parameter is undocumented/experimental, or this codebase uses a custom wrapper. To improve type safety, either update to officially supported options (env variables via configuration), add proper type definitions for custom extensions, or add a comment explaining why the cast is necessary.
| /** | ||
| * Get API environment overrides for summarization calls. | ||
| * | ||
| * Env vars (all optional): | ||
| * - EPISODIC_MEMORY_API_MODEL: Model to use (default: haiku, fallback: sonnet) | ||
| * - EPISODIC_MEMORY_API_BASE_URL: Custom API endpoint | ||
| * - EPISODIC_MEMORY_API_TOKEN: Auth token for custom endpoint | ||
| * - EPISODIC_MEMORY_API_TIMEOUT_MS: Timeout for API calls (default: SDK default) | ||
| */ | ||
| function getApiEnv(): Record<string, string> | undefined { | ||
| const baseUrl = process.env.EPISODIC_MEMORY_API_BASE_URL; | ||
| const token = process.env.EPISODIC_MEMORY_API_TOKEN; | ||
| const timeoutMs = process.env.EPISODIC_MEMORY_API_TIMEOUT_MS; | ||
|
|
||
| if (!baseUrl && !token && !timeoutMs) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const env: Record<string, string> = {}; | ||
| if (baseUrl) env.ANTHROPIC_BASE_URL = baseUrl; | ||
| if (token) env.ANTHROPIC_AUTH_TOKEN = token; | ||
| if (timeoutMs) env.API_TIMEOUT_MS = timeoutMs; | ||
| return env; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine src/summarizer.ts
fd -t f "summarizer.ts" && head -50 src/summarizer.ts && grep -n "getApiEnv\|EPISODIC_MEMORY" src/summarizer.ts | head -20Repository: obra/episodic-memory
Length of output: 2506
Remove the unsupported env parameter from the query() call.
The Claude Agent SDK's query() function does not accept an env parameter in its options. The env field is only supported in MCP server configurations for subprocess environment variables. The current code mapping environment variables to Anthropic SDK field names (ANTHROPIC_BASE_URL, ANTHROPIC_AUTH_TOKEN, API_TIMEOUT_MS) will be silently ignored by the query function. To configure API overrides, these settings must be passed to the underlying Anthropic SDK client initialization, not through query() options.
🤖 Prompt for AI Agents
In src/summarizer.ts around lines 5 to 28, the helper builds an env map
(ANTHROPIC_BASE_URL, ANTHROPIC_AUTH_TOKEN, API_TIMEOUT_MS) that is intended to
be passed to Claude SDK's query(), but query() does not accept an env option;
remove usage of env in query calls and stop generating Anthropic-specific env
vars here. Instead change getApiEnv to return a plain config object (e.g.,
baseUrl, token, timeoutMs) or otherwise surface the raw EPISODIC_* values so the
caller can initialize the Anthropic/Claude client with those overrides when
creating the SDK client; update caller code to pass those values into the client
constructor/initialization rather than into query() options.
There was a problem hiding this comment.
The env parameter IS supported by the SDK. From the type definitions:
export type Options = {
env?: { [envVar: string]: string | undefined };
// ...
}Proof it works - test with httpbin.org returned their 404 page, not a Claude response:
Result: API Error: 404 <!DOCTYPE HTML...Not Found...
However, you were partially right - our original implementation was broken. We passed only our custom vars, which replaced process.env entirely:
// BROKEN - subprocess loses PATH, HOME, etc.
env: { ANTHROPIC_BASE_URL: "..." }Fixed by merging with process.env:
// FIXED - subprocess inherits full environment
env: { ...process.env, ANTHROPIC_BASE_URL: "..." }Added a test that validates env vars are actually used (expects error from httpbin.org).
There was a problem hiding this comment.
@techjoec, thank you for the correction and for providing the type definitions and test evidence. You're absolutely right - the env parameter is supported by the SDK, and I apologize for the incorrect analysis.
I appreciate that you investigated further and found the real issue: the original implementation needed to merge with process.env to preserve essential environment variables like PATH and HOME for the subprocess. The current implementation correctly handles this:
return {
...process.env,
...(baseUrl && { ANTHROPIC_BASE_URL: baseUrl }),
...(token && { ANTHROPIC_AUTH_TOKEN: token }),
...(timeoutMs && { API_TIMEOUT_MS: timeoutMs }),
};The httpbin.org test is a clever way to validate that the custom endpoint is actually being used. The implementation looks solid now!
✏️ Learnings added
Learnt from: techjoec
Repo: obra/episodic-memory PR: 37
File: src/summarizer.ts:5-31
Timestamp: 2025-12-21T08:52:59.897Z
Learning: The Claude Agent SDK's query() function supports an `env` parameter in its Options type for passing environment variables to subprocesses. The env parameter accepts `{ [envVar: string]: string | undefined }` and should be merged with process.env to preserve essential environment variables.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
Fixed in 97ba3df - the Added test that validates env vars are actually used (hits httpbin.org and expects error response). |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/summarizer.ts (1)
58-66: Type safety bypass withas anycast.The
as anycast at line 65 bypasses TypeScript's type checking, suggesting that theenvoption may not be recognized by the SDK's type definitions. This could indicate either:
- The SDK types are incomplete/outdated
- The
envparameter is not a supported optionIf the SDK types don't include
env, this is a strong signal that the parameter may not be supported. Consider removing the cast and addressing any TypeScript errors directly.
♻️ Duplicate comments (1)
src/summarizer.ts (1)
5-31: Verify SDK support forenvparameter in query() options.A previous review flagged that the Claude Agent SDK's
query()function does not accept anenvparameter. However, the PR description claims tests pass and manual testing succeeded. This discrepancy needs verification.Please search for the latest documentation on
@anthropic-ai/claude-agent-sdkversion 0.1.9 to confirm whether theenvparameter is supported in thequery()function options:anthropic-ai claude-agent-sdk query function env parameter support
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dist/summarizer.jsis excluded by!**/dist/**
📒 Files selected for processing (2)
src/summarizer.ts(2 hunks)test/api-config.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/summarizer.ts (1)
dist/summarizer.js (6)
baseUrl(14-14)token(15-15)timeoutMs(16-16)configuredModel(42-42)defaultModel(43-43)model(44-44)
test/api-config.test.ts (2)
dist/summarizer.js (4)
result(60-60)result(121-121)result(172-172)summarizeConversation(82-179)src/summarizer.ts (1)
summarizeConversation(94-201)
🔇 Additional comments (1)
src/summarizer.ts (1)
49-51: LGTM!The model selection logic correctly implements the environment variable override while preserving the existing fallback behavior.
| import { describe, it, expect, afterEach } from 'vitest'; | ||
| import { summarizeConversation } from '../src/summarizer.js'; | ||
| import type { ConversationExchange } from '../src/types.js'; | ||
|
|
||
| describe('API Configuration', () => { | ||
| const originalEnv = { ...process.env }; | ||
|
|
||
| afterEach(() => { | ||
| // Restore original env | ||
| process.env = { ...originalEnv }; | ||
| }); | ||
|
|
||
| it('should use custom API endpoint when EPISODIC_MEMORY_API_BASE_URL is set', async () => { | ||
| // Set env vars to point to an invalid endpoint | ||
| process.env.EPISODIC_MEMORY_API_BASE_URL = 'https://httpbin.org/status/418'; | ||
| process.env.EPISODIC_MEMORY_API_TOKEN = 'test-token'; | ||
|
|
||
| const exchanges: ConversationExchange[] = [ | ||
| { | ||
| id: 'test-1', | ||
| project: 'test', | ||
| timestamp: new Date().toISOString(), | ||
| archivePath: '/test/path.jsonl', | ||
| lineStart: 1, | ||
| lineEnd: 10, | ||
| userMessage: 'Implement JWT authentication with refresh tokens', | ||
| assistantMessage: 'I will create an auth context with token rotation...' | ||
| }, | ||
| { | ||
| id: 'test-2', | ||
| project: 'test', | ||
| timestamp: new Date().toISOString(), | ||
| archivePath: '/test/path.jsonl', | ||
| lineStart: 11, | ||
| lineEnd: 20, | ||
| userMessage: 'How do protected routes work?', | ||
| assistantMessage: 'We use a ProtectedRoute component that checks auth...' | ||
| } | ||
| ]; | ||
|
|
||
| // Should fail because httpbin returns 418 "I'm a teapot" | ||
| const result = await summarizeConversation(exchanges); | ||
|
|
||
| // The result should contain an error from httpbin, proving our env vars were used | ||
| expect(result).toMatch(/API Error|418|teapot/i); | ||
| }, 30000); | ||
| }); |
There was a problem hiding this comment.
Test has multiple quality and reliability issues.
-
Process.env replacement (line 10): Using
process.env = { ...originalEnv }replaces the entire environment, potentially breaking subprocesses that need variables likePATH,HOME,USER, etc. Use selective restoration instead. -
Shallow validation (line 45): The test only checks for error keywords ("API Error|418|teapot") but doesn't verify that the custom token was actually passed to the endpoint or that the SDK correctly formatted the request as an Anthropic-compatible API call.
-
Network dependency (line 42): The test makes a real network request to httpbin.org, making it slow, flaky, and dependent on external infrastructure. Consider mocking the SDK's HTTP layer instead.
-
Incomplete coverage: The test only validates error handling, not successful routing through a custom endpoint. Consider adding a test case that validates successful summarization with environment overrides.
🔎 Proposed fix for process.env restoration
afterEach(() => {
- // Restore original env
- process.env = { ...originalEnv };
+ // Restore only the env vars we changed
+ delete process.env.EPISODIC_MEMORY_API_BASE_URL;
+ delete process.env.EPISODIC_MEMORY_API_TOKEN;
+ delete process.env.EPISODIC_MEMORY_API_MODEL;
+ delete process.env.EPISODIC_MEMORY_API_TIMEOUT_MS;
});🤖 Prompt for AI Agents
In test/api-config.test.ts lines 1-47: the test replaces the whole process.env
(line 10) and performs real network calls to httpbin.org while only doing
shallow assertions (lines ~42-47); fix by restoring env selectively (save
originalEnv and on teardown copy back only keys you modified or delete added
keys rather than assign process.env wholesale), replace the external call with a
mock of the HTTP/SDK layer (stub the fetch/axios or the internal request helper
used by summarizeConversation) so tests are fast and deterministic, add
assertions on the mocked request to verify the EPISODIC_MEMORY_API_TOKEN was
sent (e.g., Authorization header) and that the request body/headers match the
expected Anthropic-compatible API format, and add a complementary test that uses
the mock to return a successful summarization to verify routing to a custom
endpoint works.
There was a problem hiding this comment.
Fixed the process.env restoration in 7c285b7.
Re: network dependency - this is intentional. The test exists specifically to prove the env vars reach a real endpoint. Mocking would defeat the purpose of validating the SDK integration actually works.
There was a problem hiding this comment.
@techjoec, thanks for fixing the process.env restoration!
You make a fair point about the integration test — if the goal is to verify that the environment variables correctly propagate through the SDK to a real endpoint, then the network call serves that purpose. I appreciate the clarification on the test's intent.
🐰🤔
(\(\
( -.-)
o_(")(")
🧠 Learnings used
Learnt from: techjoec
Repo: obra/episodic-memory PR: 37
File: src/summarizer.ts:5-31
Timestamp: 2025-12-21T08:52:59.897Z
Learning: The Claude Agent SDK's query() function supports an `env` parameter in its Options type for passing environment variables to subprocesses. The env parameter accepts `{ [envVar: string]: string | undefined }` and should be merged with process.env to preserve essential environment variables.
The SDK's env option replaces process.env entirely, so passing only our custom vars caused the subprocess to lose PATH, HOME, etc. Now we merge our overrides with the full process.env so the spawned Claude process inherits all required environment variables. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
techjoec
force-pushed
the
feature/api-config
branch
from
97ba3df to
7c285b7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/summarizer.ts (1)
53-66: Remove the unsupported API_TIMEOUT_MS environment variable from getApiEnv().The ANTHROPIC_BASE_URL and ANTHROPIC_AUTH_TOKEN environment variables are correctly supported, but API_TIMEOUT_MS is not a valid environment variable for the Claude Agent SDK. Either remove it or replace it with a supported timeout configuration if timeout handling is needed.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dist/summarizer.jsis excluded by!**/dist/**
📒 Files selected for processing (2)
src/summarizer.ts(2 hunks)test/api-config.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/api-config.test.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: techjoec
Repo: obra/episodic-memory PR: 37
File: src/summarizer.ts:5-31
Timestamp: 2025-12-21T08:52:59.897Z
Learning: The Claude Agent SDK's query() function supports an `env` parameter in its Options type for passing environment variables to subprocesses. The env parameter accepts `{ [envVar: string]: string | undefined }` and should be merged with process.env to preserve essential environment variables.
📚 Learning: 2025-12-21T08:52:59.897Z
Learnt from: techjoec
Repo: obra/episodic-memory PR: 37
File: src/summarizer.ts:5-31
Timestamp: 2025-12-21T08:52:59.897Z
Learning: When using the Claude Agent SDK query() in TypeScript, ensure Options.env is merged with the current process.env. The env type is { [envVar: string]: string | undefined }. Create a merged env object (e.g., const env = { ...process.env, ...options.env };) and pass env to the subprocess so essential environment variables are preserved while allowing overrides.
Applied to files:
src/summarizer.ts
🧬 Code graph analysis (1)
src/summarizer.ts (1)
dist/summarizer.js (6)
baseUrl(14-14)token(15-15)timeoutMs(16-16)configuredModel(42-42)defaultModel(43-43)model(44-44)
🔇 Additional comments (2)
src/summarizer.ts (2)
5-31: LGTM - Correct implementation of env merging.The environment variable mapping and merging logic is correctly implemented. Spreading
...process.envfirst ensures the subprocess inherits essential environment variables (PATH, HOME, etc.) while allowing overrides. The conditional spreads cleanly handle optional configuration, and returningundefinedwhen no custom vars are set avoids unnecessary env manipulation.
48-51: LGTM - Clear model selection logic.The model selection correctly prioritizes
EPISODIC_MEMORY_API_MODELover theuseSonnetflag, with appropriate fallback to defaults. Passing the model as an SDK option (rather than an environment variable) is the correct approach.
|
@coderabbitai Re: "Remove the unsupported API_TIMEOUT_MS" - it is valid. 1. SDK passes env to subprocess (SDK types): env?: { [envVar: string]: string | undefined };
// "Environment variables to pass to the Claude Code process."2. CLI reads API_TIMEOUT_MS (from bundled cli.js): timeout: parseInt(process.env.API_TIMEOUT_MS || String(600000), 10)3. Reproducible test: nc -l 19991 >/dev/null & sleep 0.2
API_TIMEOUT_MS=2000 node -e "
fetch('http://127.0.0.1:19991', {signal: AbortSignal.timeout(parseInt(process.env.API_TIMEOUT_MS))})
.catch(() => console.log('Timeout: ' + Date.now() + 'ms'));"Output: Timeouts match configured values exactly. Happy to add additional tests or documentation if needed. |
|
Tip For best results, initiate chat on the files or code changes. @techjoec, you're absolutely right, and I apologize for the incorrect suggestion. Thank you for the detailed evidence demonstrating that Your breakdown clearly shows:
The 🧠 Learnings used |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3234 🏁 Script executed: Length of output: 1374 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 0 @techjoec, you're absolutely right, and I apologize for the incorrect review comment. Thank you for providing the evidence! I can confirm from the codebase that:
Your reproducible test demonstrating that the timeout values are respected (2005ms for 2000ms config, 4005ms for 4000ms config) is excellent validation. The environment variable is indeed supported by the Claude Agent SDK and works as intended. I've noted this for future reference. Thank you for the correction! 🐰 ✏️ Learnings added
🧠 Learnings used |
|
Tip For best results, initiate chat on the files or code changes. @techjoec, thank you for the thorough clarification! I've already acknowledged the correction and added this to my knowledge base. Your evidence was excellent and the implementation is correct. Regarding additional tests or documentation - the existing test in 🧠 Learnings used |
- EPISODIC_MEMORY_API_MODEL defaults to 'haiku' - EPISODIC_MEMORY_API_MODEL_FALLBACK defaults to 'sonnet' - Fix: fallback now works when primary model is configured - Log message shows actual model names instead of hardcoded 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
153-158: Consider clarifying the call count estimate for summarization.The table states "up to 10 calls/sync" for summarization. For very long conversations using hierarchical summarization, the actual call count can exceed 10 (e.g., 80 exchanges = 10 chunks + 1 synthesis = 11+ calls, potentially doubled with fallback retries). Consider rephrasing to "typically under 10 calls/sync" or "varies by conversation length" for accuracy.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
dist/mcp-server.jsis excluded by!**/dist/**dist/summarizer.jsis excluded by!**/dist/**
📒 Files selected for processing (2)
README.mdsrc/summarizer.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-21T08:52:59.897Z
Learnt from: techjoec
Repo: obra/episodic-memory PR: 37
File: src/summarizer.ts:5-31
Timestamp: 2025-12-21T08:52:59.897Z
Learning: When using the Claude Agent SDK query() in TypeScript, ensure Options.env is merged with the current process.env. The env type is { [envVar: string]: string | undefined }. Create a merged env object (e.g., const env = { ...process.env, ...options.env };) and pass env to the subprocess so essential environment variables are preserved while allowing overrides.
Applied to files:
src/summarizer.ts
📚 Learning: 2025-12-21T09:28:53.818Z
Learnt from: techjoec
Repo: obra/episodic-memory PR: 0
File: :0-0
Timestamp: 2025-12-21T09:28:53.818Z
Learning: API_TIMEOUT_MS is a valid environment variable supported by the Claude Agent SDK for configuring API call timeouts. It can be passed via the SDK's env parameter and is respected by the SDK's timeout mechanism.
Applied to files:
src/summarizer.ts
🧬 Code graph analysis (1)
src/summarizer.ts (1)
dist/summarizer.js (6)
baseUrl(15-15)token(16-16)timeoutMs(17-17)primaryModel(43-43)fallbackModel(44-44)model(45-45)
🔇 Additional comments (5)
src/summarizer.ts (5)
5-32: LGTM!The
getApiEnv()implementation correctly merges environment overrides withprocess.envto preserve essential variables (PATH, HOME, etc.) for subprocesses, as confirmed by the retrieved learnings. The mapping to Anthropic SDK environment variables (ANTHROPIC_BASE_URL, ANTHROPIC_AUTH_TOKEN, API_TIMEOUT_MS) is correct, and the optimization to returnundefinedwhen no overrides are set is appropriate.Based on learnings, the env parameter is supported by the SDK and API_TIMEOUT_MS is a valid configuration option.
49-52: LGTM!The model selection logic correctly implements the primary/fallback pattern with sensible defaults ('haiku' and 'sonnet') that match the README documentation. The
useFallbackparameter cleanly controls the retry behavior.
54-66: LGTM!The environment injection via
env: getApiEnv()correctly propagates API overrides to the SDK. The conditionalsystemPromptlogic is sound—preserving the original session's prompt when resuming (sessionIdpresent) while setting explicit instructions for new sessions. The type assertion on line 66 is acceptable given the SDK's option flexibility.
68-85: LGTM!The retry logic is well-implemented: it detects thinking budget errors specifically, retries once with the fallback model (line 75), and uses dynamic model names in logging (line 74) for clarity. The
useFallbackguard prevents infinite recursion, and error propagation is handled appropriately when the fallback also fails.
95-201: LGTM!The integration of the updated
callClaudefunction with the existingsummarizeConversationlogic is correct. ThesessionIdparameter is appropriately passed for resume support in short conversations (line 138), and the comment on lines 145-146 correctly explains why hierarchical summarization doesn't use resume mode. The overall flow remains sound.
|
awesome. thank you for this.. |
2 participants
Summary
Simplify model configuration with explicit
EPISODIC_MEMORY_API_MODELandEPISODIC_MEMORY_API_MODEL_FALLBACKenv vars.Problem
The original implementation mixed configured and default model selection in a confusing way:
This caused:
Solution
API Configuration
Changes
src/summarizer.tsREADME.mdEPISODIC_MEMORY_API_MODEL_FALLBACKTest plan
tsc --noEmitpassesnpm run buildpasses🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Behavior Changes
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.